Skip to content

Conversation

@pjonsson
Copy link
Contributor

@pjonsson pjonsson commented Oct 5, 2025

Related Issue(s):

Description:

Python import times are noticeable during
interactive use of CLI applications, so
push the (near) single-use imports into
the functions where they are used so
the import cost is paid when calling
the functions instead of when starting
the CLI to print the help or do some
other unrelated action.

Timings on top of #1583 inside
a Python:3.10-bookworm container.

Before this change:

$ uv run hyperfine --warmup 3 "python3 -c 'import pystac'" Benchmark 1: python3 -c 'import pystac'
Time (mean ± σ): 67.2 ms ± 1.6 ms [User: 58.6 ms, System: 8.6 ms]
Range (min … max): 62.5 ms … 69.9 ms 45 runs

After this change:

$ uv run hyperfine --warmup 3 "python3 -c 'import pystac'" Benchmark 1: python3 -c 'import pystac'
Time (mean ± σ): 59.9 ms ± 1.5 ms [User: 52.0 ms, System: 7.8 ms]
Range (min … max): 56.9 ms … 64.2 ms 48 runs

PR Checklist:

  • Pre-commit hooks pass (run pre-commit run --all-files)
  • Tests pass (run pytest)
  • Documentation has been updated to reflect changes, if applicable
  • This PR maintains or improves overall codebase code coverage.
  • Changes are added to the CHANGELOG. See the docs for information about adding to the changelog.

Python import times are noticeable during
interactive use of CLI applications, so
push the (near) single-use imports into
the functions where they are used so
the import cost is paid when calling
the functions instead of when starting
the CLI to print the help or do some
other unrelated action.

Timings on top of stac-utils#1583 inside
a Python:3.10-bookworm container.

Before this change:

$ uv run hyperfine --warmup 3 "python3 -c 'import pystac'"
Benchmark 1: python3 -c 'import pystac'
  Time (mean ± σ):      67.2 ms ±   1.6 ms    [User: 58.6 ms, System: 8.6 ms]
  Range (min … max):    62.5 ms …  69.9 ms    45 runs

After this change:

$ uv run hyperfine --warmup 3 "python3 -c 'import pystac'"
Benchmark 1: python3 -c 'import pystac'
  Time (mean ± σ):      59.9 ms ±   1.5 ms    [User: 52.0 ms, System: 7.8 ms]
  Range (min … max):    56.9 ms …  64.2 ms    48 runs
@gadomski gadomski self-requested a review October 6, 2025 12:48
@codecov
Copy link

codecov bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 85.71429% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.29%. Comparing base (a5e0408) to head (be238f2).

Files with missing lines Patch % Lines
pystac/validation/local_validator.py 25.00% 3 Missing ⚠️
pystac/extensions/eo.py 0.00% 1 Missing ⚠️
pystac/extensions/grid.py 0.00% 1 Missing ⚠️
pystac/extensions/projection.py 50.00% 1 Missing ⚠️
pystac/extensions/raster.py 0.00% 1 Missing ⚠️
pystac/layout.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1584      +/-   ##
==========================================
- Coverage   92.38%   92.29%   -0.09%     
==========================================
  Files          55       55              
  Lines        8375     8382       +7     
  Branches      965      965              
==========================================
- Hits         7737     7736       -1     
- Misses        453      461       +8     
  Partials      185      185              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, it feels strange to move standard library imports down into functions. What happens if you just move dateutil and other external library imports?

@guillemc23
Copy link

To me, it feels strange to move standard library imports down into functions. What happens if you just move dateutil and other external library imports?

I have seen this pattern a lot when using big imports like tensorflow, but I agree with you, I'm not sure if moving import os makes a difference 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants